Skip to content

refactor(cast): consolidate cast convert#12638

Open
dipanshuhappy wants to merge 21 commits intofoundry-rs:masterfrom
dipanshuhappy:feat/consolidate-cast-convert
Open

refactor(cast): consolidate cast convert#12638
dipanshuhappy wants to merge 21 commits intofoundry-rs:masterfrom
dipanshuhappy:feat/consolidate-cast-convert

Conversation

@dipanshuhappy
Copy link
Contributor

Motivation

Resolves #12500

Solution

Create a new file convert.rs which encapsulates all the sub commands mentioned.

PR Checklist

NOTE: This does break existing cli commands related to conversation. Refer to the issue to know more about the cli commands that will break.

  • Added Tests
  • Added Documentation
  • Breaking changes

Copy link
Collaborator

@grandizzy grandizzy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you will need to fix tests to move forward

@dipanshuhappy
Copy link
Contributor Author

thank you will need to fix tests to move forward

Updated

grandizzy
grandizzy previously approved these changes Nov 29, 2025
Copy link
Collaborator

@grandizzy grandizzy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, ty, that's much cleaner. pending other reviewers as this is a breaking change and need consensus.

@0xClandestine
Copy link
Contributor

@dipanshuhappy appreciate the work ser

Copy link
Member

@onbjerg onbjerg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should merge this as is as it is breaking. What I would suggest is shimming the existing commands to call convert <...> internally and displaying a "This command is deprecated and may be removed in the future, please use cast convert <...> instead" message.

I realize this ends up making the code a bit messy again, but I'd much prefer this to us pushing a breaking change for a very common set of commands in a non-major bump

@dipanshuhappy
Copy link
Contributor Author

dipanshuhappy commented Dec 10, 2025

I don't think we should merge this as is as it is breaking. What I would suggest is shimming the existing commands to call convert <...> internally and displaying a "This command is deprecated and may be removed in the future, please use cast convert <...> instead" message.

I realize this ends up making the code a bit messy again, but I'd much prefer this to us pushing a breaking change for a very common set of commands in a non-major bump

Understood... Updating to that ASAP.

@onbjerg onbjerg changed the title Feat/consolidate cast convert refactor(cast): consolidate cast convert Dec 11, 2025
@0xClandestine
Copy link
Contributor

curious what the status is here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

feat(cast): consolidate conversion commands

4 participants